Conversation
_matvec applied each Pauli term by recursively splitting the amplitude vector into sublists and reassembling it. Apply each term instead as a signed permutation of the amplitudes: the X and Y qubits give an index xor mask, the Y and Z qubits give the indices whose bit parity sets the sign, and each Y adds a factor of 1j. This is a pure implementation change with identical results.
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors the _matvec method in LinearQubitOperator to use bitwise operations and masks, introducing a new _bit_parity helper function to apply operators as signed permutations instead of recursively splitting vectors. The review feedback points out that using 1j ** (y_count % 4) introduces numerical inaccuracies due to floating-point approximations and suggests using a lookup table [1, 1j, -1, -1j][y_count % 4] to keep the real and imaginary parts exact.
The phase contributed by the Y factors is always one of 1, 1j, -1, -1j. Index a small table by y_count % 4 instead of evaluating 1j ** (y_count % 4). Same values, and it stays exact if the exponent is ever a numpy integer rather than a Python int.
mhucka
requested changes
Jul 16, 2026
mhucka
left a comment
Collaborator
There was a problem hiding this comment.
Thank you for this nice work! I had just a few suggestions.
Use a tuple rather than a list for the (1, 1j, -1, -1j) phase constant so it is stored once instead of rebuilt per call, keep the sign array as int8 since its values are only +1 and -1, and allocate the index array lazily so an operator with only the identity term skips it.
mhucka
approved these changes
Jul 16, 2026
mhucka
left a comment
Collaborator
There was a problem hiding this comment.
Thanks for the updates. Looks good.
mhucka
pushed a commit
to mhucka/OpenFermion
that referenced
this pull request
Jul 27, 2026
…ransform (quantumlib#1427) Third of the operator hot-path findings from quantumlib#1407; the earlier two are quantumlib#1415 and quantumlib#1417. `jordan_wigner` on an `InteractionOperator` maps the one- and two-body integrals straight to Pauli terms instead of going through a `FermionOperator` first, and quantumlib#587 points at exactly this routine as the fast path to build on ("there is a special routine that maps the InteractionOperators directly to qubits ... and that is faster"). quantumlib#121 is the other side of it: it flagged this same routine as "surprisingly slow" back in 2017 and got closed without a fix. The transform walks every one-body pair and every pair of pairs, so the two-body part is O(N^4). For each Pauli string it built a fresh single-term `QubitOperator` and summed that into the running total. Each of those constructions parses the term, validates every `(index, action)` factor, runs the Pauli `_simplify`, and in the three-unique-index case multiplies by a `Z` operator, which deepcopies. None of that is needed to add a term to a sum: a `QubitOperator`'s `terms` is just a dict from a sorted tuple of `(index, action)` factors to a coefficient, and the strings this routine emits are already sorted with their factors already combined, so there is nothing for the constructor to normalize. This accumulates the `pauli_string -> coefficient` pairs into a plain dict with a small `_add_term` helper that does what `__iadd__` / `__isub__` do to a single term (add onto the running value, drop it once it falls under `EQ_TOLERANCE`), and builds one `QubitOperator` at the end by assigning the finished dict to `.terms`. Accumulating into a dict instead of building and summing single-term operators avoids that per-term construction and validation, and it measures about 2x faster, with identical output. ### Public functions `jordan_wigner_one_body` and `jordan_wigner_two_body` are public and re-exported, and `bksf_test` and `jordan_wigner_test` call them directly, so they keep their signatures and their output. I pulled the term-building logic into two private helpers (`_one_body_terms`, `_two_body_terms`) that return the dict, and the public functions and the interaction-op driver both go through them, so there is one copy of the logic and the public results are unchanged. The `Z` times hopping-term product in the three-unique case is a single-qubit relabeling (the shared qubit carries at most a parity `Z`, and `Z` squared is the identity), so it is a direct toggle of that index in the string now rather than an operator multiply and deepcopy. ### Correctness Before editing the source I captured the current `jordan_wigner` output on 104 random real `InteractionOperator`s and saved it. The mix is physical (symmetric, molecular-style) operators, dense generic real tensors, sparse and very sparse tensors, one-body-only, constant-only, and the exact-zero operator, at N = 1, 2, 3, 4, 6, 8, 10, 12. The new code reproduces that captured output exactly on every case: identical term-key sets, max absolute coefficient difference 0, and `(new - old).induced_norm(1)` equal to 0. It also matches an independent dict-based reference I wrote separately, to the same 0. Seeds are fixed, so the check is deterministic. `jordan_wigner_test.py` and `bksf_test.py` pass (the two public helpers are checked there against the `FermionOperator` path over a full index grid with complex coefficients), and `transforms/` passes as a whole. black and mypy are clean on the file. Every changed line is hit by the existing suite except the pre-existing `n_qubits < count_qubits(iop)` guard, which the old code did not cover either and which I left untouched. The routine is still documented as real-only and this does not change that. It computes the same coefficients from the same integrals; only the container they land in changed. ### Benchmarks Random real InteractionOperators, cumulative `jordan_wigner` time. Medians over 5 repeats within each of 6 alternated upstream/branch rounds, fixed seeds, on an Intel Core Ultra 5 225 laptop (Windows, Python 3.12, numpy 2.5). N upstream ours speedup 8 16 ms 7.6 ms 2.1x 12 101 ms 48 ms 2.1x 16 359 ms 177 ms 2.0x 20 963 ms 476 ms 2.0x 24 2222 ms 1093 ms 2.0x
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Second of the operator hot-path findings from #1407; the first is #1415.
LinearQubitOperator._matvecapplies aQubitOperatorto a state vector, and itis the inner loop behind
generate_linear_qubit_operatorand the eigensolversbuilt on top of it. For each term it took the input vector, split it into
2**kpieces to reach the first non-identity qubit, split each of those in half, applied
the 2x2 Pauli to the pairs, and concatenated everything back, once per Pauli in
the term. That is a lot of Python-level list building and array copying for what
is, per term, a single reindexing of the amplitudes.
A Pauli string acts on the state vector as a signed permutation, so it can be
written directly on the flat index. Number the amplitudes
0 .. 2**n - 1and putqubit
qat bitn - 1 - q(the ordering the split version already used, and theone
qubit_operator_sparseuses). Then for one term:cmoves toc ^ x_mask;cis set,that is
(-1)raised to the parity ofpopcount(c & z_mask);1j.So the term becomes
out[idx ^ x_mask] += coeff * 1j**(y_count % 4) * sign * x,where
signis a vector of +-1 from the bit parity andidx ^ x_maskis apermutation of the indices, which is what makes the fancy-indexed
+=safe. Themasks are cheap integer folds over the term and the per-amplitude work is a few
vectorized numpy operations with no intermediate lists.
This is an implementation change only. Public behavior is unchanged: complex
output, inputs of shape
(dim,)or(dim, 1), the identity term, operatorsdeclared on more qubits than they act on, and the empty operator all behave as
before. Only the serial
_matvecis touched;ParallelLinearQubitOperatorisleft alone.
The parity of
popcountuses a small xor-fold helper on uint64 rather thannumpy.bitwise_count, because the project still supports numpy 1.26 (themax_compat CI job pins
numpy==1.26.4) andbitwise_countneeds numpy 2.0.Correctness
linear_qubit_operator_test.pypasses unchanged. On top of that I compared thenew matvec against
qubit_operator_sparse(op) @ xover random operators forn = 2..12(mixed X/Y/Z, term counts including the empty and identity-onlyoperators), real and complex
x, shapes(dim,)and(dim, 1), andn_qubitsoversized by 2, plus a
scipy.sparse.linalg.eigshrun checked against denseeigenvalues. Worst absolute differences:
black and mypy are clean on the file.
Motivation
This is what #499 asked for. That issue called for a better
_matvec, measuredthat one was worth 80-100x at system sizes of 16 qubits and up, and noted that
ParallelLinearQubitOperatorcould then be deleted as obsolete once the serialpath was fast. The sketch there routed each Pauli through Cirq's
apply_unitary; no port ever landed and the issue was closed for inactivity in2025. This PR gets the same win with plain numpy on the flat index, so it adds
no dependency, and the 120x at 16 qubits below bears out the issue's estimate.
The parallel path is also where the deadlock possibility in #1405 lives, and
with the serial matvec this fast there is rarely a reason to reach for it.
Removing it, as #499 suggested, would be a separate change; this PR fixes
neither issue.
Benchmarks
Random QubitOperators with mixed X/Y/Z terms applied to a complex state vector:
Measured on my laptop (Intel Core Ultra 5 225, Windows, Python 3.12, numpy 2.5).
I ran the base/patch comparison twice in alternation with fixed seeds, taking
medians over three runs at n = 16 and single runs at n = 18 and 20 since the old
code takes minutes there; the per-round speedups agreed within a few percent, so
the table pools both rounds.